fix(cws): register connected sockets in the flow_pid map - #54691
Conversation
flow_pid map
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76e65fb118
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
🎯 Code Coverage (details) 🔗 Commit SHA: 1af46af | Docs | Datadog PR Page | Give us feedback! |
76e65fb to
75a2484
Compare
Files inventory check summaryFile checks results against ancestor 5038adbf: Results for datadog-agent_7.84.0~devel.git.146.1af46af.pipeline.130509364-1_amd64.deb:No change detected Results for datadog-iot-agent_7.84.0~devel.git.146.1af46af.pipeline.130509364-1_amd64.deb:No change detected |
Static quality checks✅ Please find below the results from static quality gates Successful checksInfo
19 successful checks with minimal change (< 2 KiB)
|
Regression DetectorRegression Detector ResultsMetrics dashboard Baseline: 645110a Optimization Goals: ✅ No significant changes detected
|
| perf | experiment | goal | Δ mean % | Δ mean % CI | trials | links |
|---|---|---|---|---|---|---|
| ➖ | quality_gate_private_action_runner | memory utilization | +0.22 | [+0.09, +0.35] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_idle | memory utilization | +0.22 | [+0.12, +0.32] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle | memory utilization | +0.14 | [+0.02, +0.26] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_metrics_logs | memory utilization | +0.02 | [-0.22, +0.27] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_logs | % cpu utilization | +0.01 | [-0.83, +0.85] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_idle_all_features | memory utilization | -0.01 | [-0.05, +0.03] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_no_fs_load | memory utilization | -0.26 | [-0.41, -0.12] | 1 | Logs bounds checks dashboard |
| ➖ | quality_gate_security_mean_fs_load | memory utilization | -0.28 | [-0.35, -0.20] | 1 | Logs bounds checks dashboard |
Bounds Checks: ❌ Failed
| perf | experiment | bounds_check_name | replicates_passed | observed_value | links |
|---|---|---|---|---|---|
| ✅ | quality_gate_idle | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ❌ | quality_gate_idle | memory_usage | 0/10 | 172.40MiB > 154MiB | bounds checks dashboard |
| ✅ | quality_gate_idle | total_bytes_received | 10/10 | 729.33KiB ≤ 819.20KiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | intake_connections | 10/10 | 4 = 4 | bounds checks dashboard |
| ❌ | quality_gate_idle_all_features | memory_usage | 8/10 | 516.58MiB > 512MiB | bounds checks dashboard |
| ✅ | quality_gate_idle_all_features | total_bytes_received | 10/10 | 1.12MiB ≤ 1.25MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | intake_connections | 10/10 | 18 ≤ 40 | bounds checks dashboard |
| ❌ | quality_gate_logs | memory_usage | 0/10 | 205.74MiB > 195MiB | bounds checks dashboard |
| ✅ | quality_gate_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_logs | total_bytes_received | 10/10 | 264.20MiB ≤ 292MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | cpu_usage | 10/10 | 424.55 ≤ 2000 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | intake_connections | 10/10 | 15 ≤ 40 | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | memory_usage | 10/10 | 410.53MiB ≤ 430MiB | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | missed_bytes | 10/10 | 0B = 0B | bounds checks dashboard |
| ✅ | quality_gate_metrics_logs | total_bytes_received | 10/10 | 0.94GiB ≤ 1.04GiB | bounds checks dashboard |
| ✅ | quality_gate_private_action_runner | memory_usage | 10/10 | 72.38MiB ≤ 75MiB | bounds checks dashboard |
| ✅ | quality_gate_security_idle | cpu_usage | 10/10 | 31.62 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_idle | memory_usage | 10/10 | 327.00MiB ≤ 330MiB | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | cpu_usage | 10/10 | 59.73 ≤ 200 | bounds checks dashboard |
| ✅ | quality_gate_security_mean_fs_load | memory_usage | 10/10 | 302.20MiB ≤ 310MiB | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | cpu_usage | 10/10 | 20.16 ≤ 100 | bounds checks dashboard |
| ✅ | quality_gate_security_no_fs_load | memory_usage | 10/10 | 312.26MiB ≤ 320MiB | bounds checks dashboard |
Explanation
Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%
Performance changes are noted in the perf column of each table:
- ✅ = significantly better comparison variant performance
- ❌ = significantly worse comparison variant performance
- ➖ = no significant change in performance
A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".
For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:
-
Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
-
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
-
Its configuration does not mark it "erratic".
Replicate Execution Details
We run multiple replicates for each experiment/variant. However, we allow replicates to be automatically retried if there are any failures, up to 8 times, at which point the replicate is marked dead and we are unable to run analysis for the entire experiment. We call each of these attempts at running replicates a replicate execution. This section lists all replicate executions that failed due to the target crashing or being oom killed.
Note: In the below tables we bucket failures by experiment, variant, and failure type. For each of these buckets we list out the replicate indexes that failed with an annotation signifying how many times said replicate failed with the given failure mode. In the below example the baseline variant of the experiment named experiment_with_failures had two replicates that failed by oom kills. Replicate 0, which failed 8 executions, and replicate 1 which failed 6 executions, all with the same failure mode.
| Experiment | Variant | Replicates | Failure | Logs | Debug Dashboard |
|---|---|---|---|---|---|
| experiment_with_failures | baseline | 0 (x8) 1 (x6) | Oom killed | Debug Dashboard |
The debug dashboard links will take you to a debugging dashboard specifically designed to investigate replicate execution failures.
❌ Retried Profiling Replicate Execution Failures (ddprof)
Note: Profiling replicas may still be executing. See the debug dashboard for up to date status.
| Experiment | Variant | Replicates | Failure | Debug Dashboard |
|---|---|---|---|---|
| quality_gate_idle | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_idle_all_features | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | baseline | 10 | Oom killed | Debug Dashboard |
| quality_gate_metrics_logs | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_idle | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_idle | comparison | 10 | Oom killed | Debug Dashboard |
| quality_gate_security_mean_fs_load | baseline | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_mean_fs_load | comparison | 10 | Crashed (exit code: 134) | Debug Dashboard |
| quality_gate_security_no_fs_load | comparison | 10 | Crashed (exit code: 134) | Debug Dashboard |
CI Pass/Fail Decision
❌ Failed. Some Quality Gates were violated.
- quality_gate_metrics_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_metrics_logs, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check memory_usage: 8/10 replicas passed. Failed 2 which is > 0. Gate FAILED.
- quality_gate_idle_all_features, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_idle_all_features, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_idle, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_no_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_private_action_runner, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check memory_usage: 0/10 replicas passed. Failed 10 which is > 0. Gate FAILED.
- quality_gate_idle, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_idle, bounds check intake_connections: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check cpu_usage: 10/10 replicas passed. Gate passed.
- quality_gate_security_mean_fs_load, bounds check memory_usage: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check missed_bytes: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check memory_usage: 0/10 replicas passed. Failed 10 which is > 0. Gate FAILED.
- quality_gate_logs, bounds check total_bytes_received: 10/10 replicas passed. Gate passed.
- quality_gate_logs, bounds check intake_connections: 10/10 replicas passed. Gate passed.
75a2484 to
68f8c7d
Compare
b687a36 to
e650c1b
Compare
eBPF complexity changesSummary result: ✅ - stable
Groups without changes:
runtime_security detailsruntime_security [programs with changes]
runtime_security_syscall_wrapper detailsruntime_security_syscall_wrapper [programs with changes]
This report was generated based on the complexity data for the current branch yoanngh/connect-flow-pid-kernel-7 (pipeline 130509364, commit 1af46af) and the base branch main (commit 5038adb). Objects without changes are summarized by group only. Contact #ebpf-platform if you have any questions/feedback. Table complexity legend: 🔵 - new; ⚪ - unchanged; 🟢 - reduced; 🔴 - increased |
| // Before Linux 7.0 an IPv6 socket was classified on its first transmit, once its source address and port | ||
| // were known. Starting with Linux 7.0 inet6_csk_xmit only calls security_sk_classify_flow on a route miss. | ||
| __attribute__((always_inline)) int register_native_ipv6_flow(struct sock *sk, u64 pid_tgid) { | ||
| // IPv4 and ipv4 mapped addresses still reach security_sk_classify_flow with a usable flow |
There was a problem hiding this comment.
The comment is correct but not really clear: this function does not need to do anything for two different cases:
- for a socket using IPv4
- for a socket using IPv6 but using IPv4 addresses
I split the comment into two parts to make that more clear
security_sk_classify_flow runs in tcp_v6_connect before the ephemeral source port and the source address have been picked, so the flow it reports for a connecting socket is incomplete. Until now the transmit path called the hook again once both were known; since Linux 7.0 inet6_csk_xmit only routes on a dst cache miss, so that second call never happens and IPv6 TCP client flows are either registered under a wildcard address or, when the socket was never bound, not registered at all. Register the flow on the connect syscall exit instead, reading the final port and source address from the socket. The connect probes are now loaded along with the network probes so that flow_pid stays up to date even when no rule asks for connect events.
6dc955e to
1af46af
Compare
What does this PR do?
Registers the flow of a connecting IPv6 socket once
connectreturns.Motivation
tcp_v6_connectclassifies the flow before the ephemeral source port and the source address are picked, so until Linux 7.0 the socket was classified on its first transmit, once both were known. Linux 7.0 only routes on a dst cache miss ininet6_csk_xmit, and a connecting socket always ends up holding a route, so that second classification never happens and those flows are left unattributed.Describe how you validated your changes
Existing functional tests running on Ubuntu 26.04 that this PR fixes.
Additional Notes
IPv4 flows are unaffected as these are still registered as part of the
security_sk_classify_flowhook.